Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[EASI-4658] Integrate discussions tagging #2917

Merged
merged 11 commits into from
Dec 11, 2024
Merged

[EASI-4658] Integrate discussions tagging #2917

merged 11 commits into from
Dec 11, 2024

Conversation

aterstriep
Copy link
Contributor

EASI-4658

Figma

Description

  • Integrated discussions tagging functionality
  • Added types to files in MentionTextArea component folder

How to test this change

  • Test tagging functionality on both start a discussion and reply forms

PR Author Checklist

  • I have provided a detailed description of the changes in this PR.
  • I have provided clear instructions on how to test the changes in this PR.
  • I have updated tests or written new tests as appropriate in this PR.

PR Reviewer Guidelines

  • It's best to pull the branch locally and test it, rather than just looking at the code online!
  • When approving a PR, provide a reason why you're approving it
    • e.g. "Approving because I tested it locally and all functionality works as expected"
    • e.g. "Approving because the change is simple and matches the Figma design"
  • Don't be afraid to leave comments or ask questions, especially if you don't understand why something was done! (This is often a great time to suggest code comments or documentation updates)
  • Check that all code is adequately covered by tests - if it isn't feel free to suggest the addition of tests.

@aterstriep
Fix ID bug
6ab1256 
Removed duplicate ID on container in previous PR. Caused a bug where mention suggestions box does not render correctly. Replacing duplicate ID prop for now.
@aterstriep
Rename reviewers query
c3b08b1
@aterstriep
Replace mock mention suggestions with grbReviewers
06ae4ca
@aterstriep
Fix duplicate ID on EditorContent
1e19058 
Use editor element instead of ID to append suggestions popup
@aterstriep
Typed MentionTextArea component files
70a3e62
@aterstriep
Add data-label attr for rendering mentions
95775c7
@aterstriep
Merge branch 'feature/EASI-4614_grb_discussions' into EASI-4658/integ…
a0aa4c6 
…rate-tagging-discussions
aterstriep
aterstriep commented Dec 11, 2024
View reviewed changes
pkg/sanitization/tagged_html.go
@@ -31,6 +31,6 @@ func createTaggedHTMLPolicy() *bluemonday.Policy {
policy := bluemonday.NewPolicy()
// rules for tags
policy.AllowElements("span", "p")
policy.AllowAttrs("data-type", "class", "tag-type", "data-id-db").OnElements("span")
policy.AllowAttrs("data-type", "class", "tag-type", "data-id-db", "data-label").OnElements("span")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiptap needs a data-label attribute to render the mention labels. Without this attribute, the mentions just rendered as an empty span tag.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!! 🔥

aterstriep
aterstriep commented Dec 11, 2024
View reviewed changes
src/components/MentionTextArea/suggestion.ts
getReferenceClientRect: getClientRect(props),
appendTo: () => document.getElementById(editorID) || document.body,
appendTo: props.editor.options.element,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this to use the editor element instead of getting it from the id prop. This way we don't have to have duplicate id attributes on both the editor and container.

@github-actions GitHub Actions
Copy link

EKS Ingress URLs

@ClayBenson94
Copy link
Collaborator

Doing some local testing and things seem to work as expected 👀

image
image

@aterstriep aterstriep marked this pull request as ready for review December 11, 2024 15:18
@aterstriep aterstriep requested review from a team as code owners December 11, 2024 15:18
@aterstriep aterstriep requested review from mynar7 and adamodd and removed request for a team December 11, 2024 15:18
patrickseguraoddball
patrickseguraoddball requested changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@patrickseguraoddball patrickseguraoddball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall really awesome work and works great :)

This seems to be displaying the oldest discussion, not the most recent. It doesn't seem to be related to cache/refresh. Also looks like we want to get the latest discussion or reply. If the latest activity is a reply, render that discussion as the latest activity?
Screenshot 2024-12-11 at 10 40 51 AM

easi-app/src/views/GovernanceReviewTeam/GRBReview/Discussions.tsx

Line 45 in 543ad8a

const recentDiscussion =

Getting some translation key instead of translated text
Screenshot 2024-12-11 at 10 17 26 AM

Doesn't seem to have an existing i18n namespace

easi-app/src/components/MentionTextArea/MentionList.tsx

Line 38 in 543ad8a

const { t } = useTranslation('discussionsMisc');

@ClayBenson94
Copy link
Collaborator

ClayBenson94 commented Dec 11, 2024 •
edited
Loading

If the latest activity is a reply, render that discussion as the latest activity?

@patrickseguraoddball @aterstriep Maybe we should figure out how this should work with Zoë and Jeff, since the # of replies beneath it might imply it's only ever meant to show the most recent top-level post?

@aterstriep
Copy link
Contributor Author

Maybe we should figure out how this should work with Zoë and Jeff

@ClayBenson94 @patrickseguraoddball Slack thread here!

@aterstriep
Copy link
Contributor Author

This seems to be displaying the oldest discussion

@patrickseguraoddball @ClayBenson94 this is fixed!

patrickseguraoddball
patrickseguraoddball approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@patrickseguraoddball patrickseguraoddball left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG Thanks for getting that recent activity resolved!

@aterstriep aterstriep merged commit ce5a0be into feature/EASI-4614_grb_discussions Dec 11, 2024
14 checks passed
@aterstriep aterstriep deleted the EASI-4658/integrate-tagging-discussions branch December 11, 2024 23:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samoddball samoddball samoddball left review comments

@patrickseguraoddball patrickseguraoddball patrickseguraoddball approved these changes

@mynar7 mynar7 Awaiting requested review from mynar7 mynar7 is a code owner automatically assigned from CMS-Enterprise/easi-backend

@adamodd adamodd Awaiting requested review from adamodd adamodd is a code owner automatically assigned from CMS-Enterprise/easi-frontend

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aterstriep @ClayBenson94 @patrickseguraoddball @samoddball